-
-
Notifications
You must be signed in to change notification settings - Fork 45
Complete source generators for queries #84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Introduces the ApplyFilterGenerator Roslyn source generator to generate efficient ApplyFilter extension methods at compile time, reducing reflection overhead. Adds comprehensive documentation for query generation flow and source generator usage. Provides sample DTOs, models, and test scaffolding to demonstrate and validate generator functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a comprehensive Roslyn source generator (ApplyFilterGenerator) to generate compile-time filter application code, eliminating reflection overhead and enabling AOT compatibility. It includes extensive documentation, test infrastructure with sample models/filters, and sandbox examples to demonstrate the generator's functionality.
- Adds
ApplyFilterGeneratorto generate efficientApplyFilterextension methods at compile time - Introduces comprehensive test infrastructure with sample models, filters, and smoke tests
- Provides detailed documentation covering generator usage, features, and query generation flow
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
tests/AutoFilterer.Generators.Tests/TestDataHelper.cs |
Test data factory for Books, Authors, and Publishers with navigation relationships |
tests/AutoFilterer.Generators.Tests/GeneratorSmokeTests.cs |
Smoke tests validating generated code compiles and executes correctly for various filter types |
tests/AutoFilterer.Generators.Tests/FilterGeneratorTests.cs |
Refactored existing tests into separate namespace to avoid naming conflicts |
tests/AutoFilterer.Generators.Tests/Environment/Models/TestModels.cs |
Entity models (Book, Author, Publisher) for testing |
tests/AutoFilterer.Generators.Tests/Environment/Dtos/TestFilters.cs |
Comprehensive filter DTOs testing all generator features |
tests/AutoFilterer.Generators.Tests/AutoFilterer.Generators.Tests.csproj |
Project configuration updated to net10.0 target |
src/AutoFilterer.Generators/ApplyFilterGenerator.cs |
Core incremental source generator implementing filter-to-LINQ translation |
sandbox/WebApplication.API/Models/Author.cs |
Sample entity model for sandbox demo |
sandbox/WebApplication.API/Dtos/AuthorFilterWithGenerator.cs |
Sample filter DTO demonstrating generator usage |
docs/en/generators/SourceGenerator-ApplyFilter.md |
Complete user guide for the ApplyFilter source generator |
QUERY_GENERATION_FLOW.md |
Technical deep-dive into query generation architecture and flow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sb.AppendLine("using System;"); | ||
| sb.AppendLine("using System.Linq;"); | ||
| sb.AppendLine("using AutoFilterer.Abstractions;"); | ||
| sb.AppendLine("using AutoFilterer;"); |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The generated code references 'AutoFilterer.Sorting' namespace at line 244, but the using statements don't include 'using AutoFilterer.Enums;'. Depending on the LEGACY_NAMESPACE flag, the Sorting enum is in either 'AutoFilterer.Enums' or 'AutoFilterer' namespace. Since line 113 includes 'using AutoFilterer;', this should work for non-legacy builds, but for projects with LEGACY_NAMESPACE defined, the generated code will fail to compile. Consider adding 'using AutoFilterer.Enums;' to the generated using statements to ensure compatibility with both build configurations.
| sb.AppendLine("using AutoFilterer;"); | |
| sb.AppendLine("using AutoFilterer;"); | |
| sb.AppendLine("using AutoFilterer.Enums;"); |
| var books = GetSampleBooks(); | ||
| foreach (var author in authors) | ||
| { | ||
| author.Books = books.Where(b => b.AuthorId == author.Id).ToList(); | ||
| foreach (var book in author.Books) | ||
| { | ||
| book.Author = author; | ||
| } | ||
| } |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling GetSampleBooks() creates a new list of Book instances each time, but these are then shared across all Author instances. When setting book.Author = author on line 41, this modifies the shared Book instances. This could lead to unexpected behavior if GetSampleAuthors is called multiple times in a test, as the Book.Author reference will point to the last Author that was assigned. Consider creating independent Book instances for each Author, or documenting that this method should only be called once per test to establish the relationships.
| var isNullable = filterProp.NullableAnnotation == Microsoft.CodeAnalysis.NullableAnnotation.Annotated; | ||
| if (isNullable) | ||
| { | ||
| sb.AppendLine($" if (filter.{fpName} != null) source = source.Where(x => x.{epName}.Equals(filter.{fpName}));"); |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The generated code for nullable value type comparisons uses x.{epName}.Equals(filter.{fpName}), but if the entity property is nullable and the filter value is not null, this will cause a compilation error because it's calling Equals on a nullable type without null-checking. The correct code should be either 'x.{epName} == filter.{fpName}' (which handles nullable comparison correctly) or add a null check like 'x.{epName}.HasValue && x.{epName}.Value.Equals(filter.{fpName}.Value)'.
| sb.AppendLine($" if (filter.{fpName} != null) source = source.Where(x => x.{epName}.Equals(filter.{fpName}));"); | |
| sb.AppendLine($" if (filter.{fpName} != null) source = source.Where(x => x.{epName} == filter.{fpName});"); |
| var sortByProp = filterSymbol.GetMembers().OfType<IPropertySymbol>().FirstOrDefault(p => p.Name == "SortBy"); | ||
| if (sortByProp != null) | ||
| { | ||
| sb.AppendLine(" if (filter.SortBy == AutoFilterer.Sorting.Descending)"); |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The generated code references 'AutoFilterer.Sorting.Descending', but based on the Sorting enum definition, the namespace should be 'AutoFilterer.Enums.Sorting' (or just 'AutoFilterer' depending on the LEGACY_NAMESPACE flag). The tests use 'AutoFilterer.Enums.Sorting.Descending' (line 89 in GeneratorSmokeTests.cs), which suggests the generated code should match this pattern. This will cause compilation errors in the generated code.
| sb.AppendLine(" if (filter.SortBy == AutoFilterer.Sorting.Descending)"); | |
| sb.AppendLine(" if (filter.SortBy == AutoFilterer.Enums.Sorting.Descending)"); |
| var fpName = filterProp.Name; | ||
| var epName = entityProp.Name; | ||
|
|
||
| // Get FilterOption (Any or All) | ||
| var filterOption = "Any"; // default | ||
| var filterOptionArg = collectionFilterAttr.ConstructorArguments.Length > 0 ? collectionFilterAttr.ConstructorArguments[0] : default; | ||
| if (filterOptionArg.Value != null) | ||
| { | ||
| // The value is an integer enum value | ||
| var enumValue = filterOptionArg.Value.ToString(); | ||
| filterOption = enumValue == "0" ? "Any" : "All"; | ||
| } | ||
| else | ||
| { | ||
| var namedArg = collectionFilterAttr.NamedArguments.FirstOrDefault(kv => kv.Key == "FilterOption").Value; | ||
| if (namedArg.Value != null) | ||
| { | ||
| var enumValue = namedArg.Value.ToString(); | ||
| filterOption = enumValue == "0" ? "Any" : "All"; | ||
| } | ||
| } | ||
|
|
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for extracting filter option (Any/All) from CollectionFilterAttribute is duplicated in three places: EmitCollectionFilter (lines 300-316) and BuildNestedCollectionCondition (lines 531-546). Consider extracting this into a shared helper method like GetCollectionFilterOption that takes an AttributeData and returns the filter option string. This would reduce duplication and make maintenance easier.
| var fpName = filterProp.Name; | |
| var epName = entityProp.Name; | |
| // Get FilterOption (Any or All) | |
| var filterOption = "Any"; // default | |
| var filterOptionArg = collectionFilterAttr.ConstructorArguments.Length > 0 ? collectionFilterAttr.ConstructorArguments[0] : default; | |
| if (filterOptionArg.Value != null) | |
| { | |
| // The value is an integer enum value | |
| var enumValue = filterOptionArg.Value.ToString(); | |
| filterOption = enumValue == "0" ? "Any" : "All"; | |
| } | |
| else | |
| { | |
| var namedArg = collectionFilterAttr.NamedArguments.FirstOrDefault(kv => kv.Key == "FilterOption").Value; | |
| if (namedArg.Value != null) | |
| { | |
| var enumValue = namedArg.Value.ToString(); | |
| filterOption = enumValue == "0" ? "Any" : "All"; | |
| } | |
| } | |
| string GetCollectionFilterOption(AttributeData collectionFilterAttr) | |
| { | |
| var filterOption = "Any"; // default | |
| var filterOptionArg = collectionFilterAttr.ConstructorArguments.Length > 0 | |
| ? collectionFilterAttr.ConstructorArguments[0] | |
| : default; | |
| if (filterOptionArg.Value != null) | |
| { | |
| // The value is an integer enum value | |
| var enumValue = filterOptionArg.Value.ToString(); | |
| filterOption = enumValue == "0" ? "Any" : "All"; | |
| } | |
| else | |
| { | |
| var namedArg = collectionFilterAttr.NamedArguments | |
| .FirstOrDefault(kv => kv.Key == "FilterOption") | |
| .Value; | |
| if (namedArg.Value != null) | |
| { | |
| var enumValue = namedArg.Value.ToString(); | |
| filterOption = enumValue == "0" ? "Any" : "All"; | |
| } | |
| } | |
| return filterOption; | |
| } | |
| var fpName = filterProp.Name; | |
| var epName = entityProp.Name; | |
| // Get FilterOption (Any or All) | |
| var filterOption = GetCollectionFilterOption(collectionFilterAttr); |
| // For each CompareTo target property, emit comparisons | ||
| foreach (var attr in compareToAttrs2) | ||
| { | ||
| var combineArg = attr.NamedArguments.FirstOrDefault(kv => kv.Key == "CombineWith").Value; | ||
| var combineWith = combineArg.Value?.ToString() ?? "Or"; | ||
|
|
||
| var propNamesConst = attr.ConstructorArguments.Length > 0 ? attr.ConstructorArguments[0] : default; | ||
| if (propNamesConst.Kind == TypedConstantKind.Array) | ||
| { | ||
| foreach (var item in propNamesConst.Values) | ||
| { | ||
| var targetName = item.Value?.ToString(); | ||
| if (string.IsNullOrWhiteSpace(targetName)) continue; | ||
| var entityProp = ResolveMemberByPath(entitySymbol, targetName); | ||
| if (entityProp == null) continue; | ||
| EmitScalarComparison(sb, entitySymbol, prop, entityProp, combineWith); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| var targetName = propNamesConst.Value?.ToString(); | ||
| if (string.IsNullOrWhiteSpace(targetName)) continue; |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for extracting target property names from CompareToAttribute constructor arguments is duplicated across multiple locations (lines 205-216, 433-447, 487-501). This pattern of checking for array vs. single value and extracting target names appears at least three times. Consider extracting this into a shared helper method like ExtractTargetPropertyNames that takes a TypedConstant and returns a List of string. This would improve maintainability and reduce the risk of inconsistent behavior.
| // For each CompareTo target property, emit comparisons | |
| foreach (var attr in compareToAttrs2) | |
| { | |
| var combineArg = attr.NamedArguments.FirstOrDefault(kv => kv.Key == "CombineWith").Value; | |
| var combineWith = combineArg.Value?.ToString() ?? "Or"; | |
| var propNamesConst = attr.ConstructorArguments.Length > 0 ? attr.ConstructorArguments[0] : default; | |
| if (propNamesConst.Kind == TypedConstantKind.Array) | |
| { | |
| foreach (var item in propNamesConst.Values) | |
| { | |
| var targetName = item.Value?.ToString(); | |
| if (string.IsNullOrWhiteSpace(targetName)) continue; | |
| var entityProp = ResolveMemberByPath(entitySymbol, targetName); | |
| if (entityProp == null) continue; | |
| EmitScalarComparison(sb, entitySymbol, prop, entityProp, combineWith); | |
| } | |
| } | |
| else | |
| { | |
| var targetName = propNamesConst.Value?.ToString(); | |
| if (string.IsNullOrWhiteSpace(targetName)) continue; | |
| static List<string> ExtractTargetPropertyNames(TypedConstant propNamesConst) | |
| { | |
| var result = new List<string>(); | |
| if (propNamesConst.Kind == TypedConstantKind.Array) | |
| { | |
| foreach (var item in propNamesConst.Values) | |
| { | |
| var targetName = item.Value?.ToString(); | |
| if (!string.IsNullOrWhiteSpace(targetName)) | |
| { | |
| result.Add(targetName); | |
| } | |
| } | |
| } | |
| else | |
| { | |
| var targetName = propNamesConst.Value?.ToString(); | |
| if (!string.IsNullOrWhiteSpace(targetName)) | |
| { | |
| result.Add(targetName); | |
| } | |
| } | |
| return result; | |
| } | |
| // For each CompareTo target property, emit comparisons | |
| foreach (var attr in compareToAttrs2) | |
| { | |
| var combineArg = attr.NamedArguments.FirstOrDefault(kv => kv.Key == "CombineWith").Value; | |
| var combineWith = combineArg.Value?.ToString() ?? "Or"; | |
| var propNamesConst = attr.ConstructorArguments.Length > 0 ? attr.ConstructorArguments[0] : default; | |
| var targetNames = ExtractTargetPropertyNames(propNamesConst); | |
| foreach (var targetName in targetNames) | |
| { |
| foreach (var iface in type.AllInterfaces) | ||
| { | ||
| if (iface.Name == "IFilter" && iface.ContainingNamespace?.ToDisplayString() == "AutoFilterer.Abstractions") | ||
| { | ||
| return true; | ||
| } |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
| foreach (var iface in type.AllInterfaces) | |
| { | |
| if (iface.Name == "IFilter" && iface.ContainingNamespace?.ToDisplayString() == "AutoFilterer.Abstractions") | |
| { | |
| return true; | |
| } | |
| var implementsIFilter = type.AllInterfaces.Any(iface => | |
| iface.Name == "IFilter" && | |
| iface.ContainingNamespace?.ToDisplayString() == "AutoFilterer.Abstractions"); | |
| if (implementsIFilter) | |
| { | |
| return true; |
| foreach (var item in propNamesConst.Values) | ||
| { | ||
| var targetName = item.Value?.ToString(); | ||
| if (string.IsNullOrWhiteSpace(targetName)) continue; | ||
| var entityProp = ResolveMemberByPath(entitySymbol, targetName); | ||
| if (entityProp != null) | ||
| { | ||
| EmitCollectionFilter(sb, entitySymbol, prop, entityProp, collectionFilterAttr, "x"); | ||
| } | ||
| } |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| foreach (var item in propNamesConst.Values) | ||
| { | ||
| var targetName = item.Value?.ToString(); | ||
| if (string.IsNullOrWhiteSpace(targetName)) continue; | ||
| var entityProp = ResolveMemberByPath(entitySymbol, targetName); | ||
| if (entityProp == null) continue; |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| foreach (var item in propNamesConst.Values) | |
| { | |
| var targetName = item.Value?.ToString(); | |
| if (string.IsNullOrWhiteSpace(targetName)) continue; | |
| var entityProp = ResolveMemberByPath(entitySymbol, targetName); | |
| if (entityProp == null) continue; | |
| var entityProps = propNamesConst.Values | |
| .Select(v => v.Value?.ToString()) | |
| .Where(targetName => !string.IsNullOrWhiteSpace(targetName)) | |
| .Select(targetName => ResolveMemberByPath(entitySymbol, targetName)) | |
| .Where(entityProp => entityProp != null); | |
| foreach (var entityProp in entityProps) | |
| { |
| { | ||
| Assert.NotNull(typeof(MappingTest.AllTypesTestTypeFilter)); | ||
|
|
||
| var filter = new MappingTest.AllTypesTestTypeFilter(); |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assignment to filter is useless, since its value is never read.
| var filter = new MappingTest.AllTypesTestTypeFilter(); | |
| var filter = new MappingTest.AllTypesTestTypeFilter(); | |
| Assert.NotNull(filter); |
Introduces the ApplyFilterGenerator Roslyn source generator to generate efficient ApplyFilter extension methods at compile time, reducing reflection overhead. Adds comprehensive documentation for query generation flow and source generator usage. Provides sample DTOs, models, and test scaffolding to demonstrate and validate generator functionality.